-
-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add option to change the filename #51
feat: add option to change the filename #51
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronen-e Also Docs && Tests please :)
index.js
Outdated
@@ -11,6 +11,7 @@ function CompressionPlugin(options) { | |||
options = options || {}; | |||
this.asset = options.asset || "[path].gz[query]"; | |||
this.algorithm = options.algorithm || "gzip"; | |||
this.changeNewFileName = options.changeNewFileName || false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.changeNewFileName
=> this.name
or something that is similiar terse 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See how we did something similar in extract-text-webpack-plugin. Maybe a good solution to mimic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronen-e please see https://github.com/webpack-contrib/extract-text-webpack-plugin#options (options.filename
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michael-ciniawsky I didn't see any tests for the existing code.
I would like to add to them if they exist.
Do you have any tests that you already run on the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah... We would have to update this project to webpack-defaults to get testing infrastructure in place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronen-e Ooops.. 😛 forget what I said then 🙃 Just change tofilename
+ docs and good to go imho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronen-e Thx
Optionally change file name